fix(arrow): error on required field absent from data file with no default#2797
fix(arrow): error on required field absent from data file with no default#2797mbutrovich wants to merge 2 commits into
Conversation
…ault RecordBatchTransformer resolved a projected field missing from a data file by always producing a null column when it had no initial-default. That is correct only for optional fields. Per the spec's Default values section, null is a valid default only for optional fields, so a required field that is absent with no initial-default has no valid value and must error rather than producing a null column that violates its non-nullable constraint. Mirror Iceberg-Java's Parquet readers (BaseParquetReaders / SparkParquetReaders): resolve to the file value, then initial-default, then null if optional, otherwise raise "Missing required field: <name>".
viirya
left a comment
viirya
left a comment
There was a problem hiding this comment.
Verified against both the spec and Java, and ran the change locally (applied on current main): all 12 record_batch_transformer tests pass, including the new one.
The Java match is exact: BaseParquetReaders.defaultReader resolves file value → initial-default → nulls-if-optional → throw ... "Missing required field: %s", and it fires at reader-construction time — same precedence and message as this change.
One spec nuance worth recording here so nobody later cites the spec to revert this: the reader-side Column Projection section literally ends with "Return null in all other cases" — the spec never explicitly mandates a reader error for a missing required field (the "must fail" language in Default values is writer-scoped). Erroring is Java's implementation choice resolving the tension between those two sections, and following Java is the right call; just worth being precise that this matches the reference implementation rather than an explicit spec requirement.
Non-blocking residual I probed while reviewing: a required field whose initial_default is non-primitive (e.g. a struct default, V3) passes the new initial_default.is_none() check, but the extraction right below drops non-primitive literals (if let Literal::Primitive ... else None), so it still produces a NULL column and dies with the old confusing Arrow error — I reproduced Column 's' is declared as non-nullable but contains null values with a struct-defaulted required field. Since non-primitive defaults aren't otherwise supported in this path yet, fine to leave for the V3-defaults follow-up, but the check could alternatively be "error when required and the default cannot be materialized" to close it now. LGTM either way.
2 participants
Which issue does this PR close?
What changes are included in this PR?
When a projected field is not present in a data file,
RecordBatchTransformerresolves it using the spec's Column Projection fallback rules. For a field with noinitial-defaultit produced a null column (rule #4) unconditionally. That is correct only for optional fields.Per the spec's Default values section, null is a valid default only for an optional field:
So a required field that is absent from a data file with no
initial-defaulthas no valid value, and producing a null column for it violates the field's required (non-nullable) constraint. Previously this surfaced later as a confusing Arrow error (Column '<name>' is declared as non-nullable but contains null values) instead of a clear one.This PR adds the required-field case to the "not present" fallback in
record_batch_transformer.rs: if a field is absent from the data file, has noinitial-default, and is required, returnMissing required field: <name>. The resolution order now mirrors Iceberg-Java's Parquet readers (BaseParquetReaders/SparkParquetReaders.defaultReader): file value, theninitial-default, then null if optional, otherwise error.Are these changes tested?
Yes, a unit test in
record_batch_transformer.rs. Together with the existingtest_all_four_spec_rulesandschema_evolution_adds_date_column_with_nulls, all combinations of a field absent from the data file are covered:initial-defaultreturns the default value (rule Discussion: The design of in memory model of iceberg spec. #3) - existingtest_all_four_spec_rules.test_all_four_spec_rulesandschema_evolution_adds_date_column_with_nulls.Missing required field: <name>- newschema_evolution_required_field_absent_without_default_errors.